-
Notifications
You must be signed in to change notification settings - Fork 10
Doc style #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doc style #113
Conversation
|
@bcalford Have a look at #105 and its diff. The renaming of the modules and the addition of the docstring has been done there and it is ready to merge. I am just waiting for @hanayik to add a codecov token to IPyNiiVue so that the coverage reported is added to the PRs. So, I am afraid that some of the work you did here is a duplicate of what I did in #105. However, the docstring that I made in #105 are less detailed than those that you add or modify in this PR. I added just one-liners wherever they were missing just to get the CI to pass the tests. I suggest you leave this PR aside for now, wait for #105 to merge, and then rebase against the main. If it is blocking you, I can merge #105 now and figure out the codecov after... the coverage is added anyway, it is just the report that is not displayed because I cannot add the codecov token to the repo. |
|
@christian-oreilly This PR isn’t blocking anything for me, so no rush in merging #105. The doc strings put in there look nice. I will review any differences between the ones here and in #105 after it merges and then add the elaboration on any classes/functions I think may need it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am good with the state of that PR. @bcalford you can merge if you don't want to add anything else to that PR. Please squash the commits during the merging.
Adds doc string to all of the modules that have been switched from private to public.
@christian-oreilly Options_mixin.py is currently left without docstring. This file is generated by another and has a warning not to edit it directly. Docstring shouldn't mess with anything, though. Should I add the docstrings directly to that file, or should I find a way to add them when generated with the generate_options_mixin.py file?
Closes #109